refactor(BA-5710): thread main_access_key through UserPermission#11043
Closed
jopemachine wants to merge 5 commits into
Closed
refactor(BA-5710): thread main_access_key through UserPermission#11043jopemachine wants to merge 5 commits into
jopemachine wants to merge 5 commits into
Conversation
github-actions
Bot
added
size:M
This was referenced Apr 14, 2026
jopemachine
changed the title
This was referenced Apr 14, 2026
jopemachine
force-pushed
the
refactor/BA-5650-A-main-access-key-helpers
branch
from
2d7d64e to
e66536b
Compare
jopemachine
force-pushed
the
refactor/BA-5650-B-user-permission-owner-id
branch
from
6d34e21 to
d88a14b
Compare
jopemachine
commented
Apr 14, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the BA-5650 refactor stack by changing kernel-level permission data to carry the owning user ID (owner_id) and the user’s main_access_key (nullable), and wiring kernel query paths to load the associated user row so KernelInfo can be populated without relying on kernels.access_key.
Changes:
- Update
UserPermissionto useowner_idandmain_access_key: str | None. - Populate
KernelInfo.user_permission.main_access_keyfromKernelRow.user_row.main_access_key, and eager-loadKernelRow.user_rowin session kernel search. - Update selected unit-test fixtures to the new permission field names, and add a changelog entry.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/sokovan/scheduler/terminator/conftest.py | Updates termination fixture data to renamed access-key field(s) and permission fields. |
| tests/unit/manager/sokovan/scheduler/handlers/conftest.py | Updates scheduler handler fixtures for renamed permission/access-key fields and session owner naming. |
| tests/unit/manager/services/session/test_session_service.py | Updates UserPermission construction in a kernel info test helper. |
| tests/unit/manager/api/compute_sessions/test_handler.py | Updates session/kernel test factories to renamed owner/access-key fields. |
| src/ai/backend/manager/repositories/session/db_source/db_source.py | Eager-loads KernelRow.user_row during kernel search to support new KernelInfo mapping. |
| src/ai/backend/manager/models/kernel/row.py | Drops access_key from delegate_ownership, maps KernelInfo permissions via user_row.main_access_key, and changes concurrency recalculation filtering. |
| src/ai/backend/manager/data/kernel/types.py | Renames UserPermission fields to owner_id + main_access_key. |
| changes/11043.misc.md | Adds a changelog note describing the refactor intent and behavior. |
Comments suppressed due to low confidence (7)
tests/unit/manager/sokovan/scheduler/handlers/conftest.py:114
SessionMetadata(fromai.backend.manager.data.session.types) currently hasuser_uuidandaccess_keyfields. This fixture now passesowner_idand omitsaccess_key, which will raise aTypeErrorwhen constructingSessionMetadata. This PR only changesUserPermission, so the session metadata fields should remain aligned withSessionMetadatauntil its rename lands.
metadata=SessionMetadata(
name=f"session-{sid}",
domain_name="default",
group_id=group_id,
user_uuid=user_uuid,
access_key=access_key,
session_type=session_type,
priority=0,
created_at=now,
tag=None,
tests/unit/manager/sokovan/scheduler/handlers/conftest.py:629
SessionDataForStartstill usesaccess_keyanduser_uuidfields (seesokovan/data/lifecycle.py). This factory passesmain_access_keyandowner_id, so it will raise aTypeError. Keep the argument names aligned with the current dataclass (or move the type rename into this PR).
sessions_for_start = [
SessionDataForStart(
session_id=s.session_info.identity.id,
creation_id=s.session_info.identity.creation_id,
access_key=AccessKey(s.session_info.metadata.access_key),
session_type=s.session_info.identity.session_type,
name=s.session_info.identity.name,
cluster_mode=ClusterMode(s.session_info.resource.cluster_mode),
kernels=[
KernelBindingData(
kernel_id=KernelId(k.id),
agent_id=AgentId(k.resource.agent) if k.resource.agent else None,
agent_addr=k.resource.agent_addr,
scaling_group=s.session_info.resource.scaling_group_name or "default",
image="test-image:latest",
architecture=k.image.architecture or "x86_64",
)
for k in s.kernel_infos
],
user_uuid=s.session_info.metadata.user_uuid,
user_email="test@example.com",
user_name="test-user",
tests/unit/manager/sokovan/scheduler/handlers/conftest.py:663
TerminatingSessionDatastill defines the field asaccess_key, notmain_access_key(seerepositories/scheduler/types/session.py). This factory will fail when constructingTerminatingSessionDatawith the renamed argument.
return [
TerminatingSessionData(
session_id=s.session_info.identity.id,
access_key=AccessKey(s.session_info.metadata.access_key),
tests/unit/manager/api/compute_sessions/test_handler.py:75
SessionData(fromai.backend.manager.data.session.types) still usesuser_uuidand includes anaccess_keyfield. This test factory now passesowner_idand omitsaccess_key, which will raise aTypeErrorwhen constructingSessionData. Either keep the factory aligned with the currentSessionDatasignature, or include theSessionDatarename in the same PR.
return SessionData(
id=session_id or uuid4(),
session_type=SessionTypes.INTERACTIVE,
priority=0,
is_preemptible=True,
cluster_mode=ClusterMode.SINGLE_NODE,
cluster_size=1,
domain_name="default",
group_id=uuid4(),
user_uuid=uuid4(),
occupying_slots=ResourceSlot({"cpu": Decimal("2.0"), "mem": Decimal("4294967296")}),
requested_slots=ResourceSlot({"cpu": Decimal("4.0"), "mem": Decimal("8589934592")}),
tests/unit/manager/sokovan/scheduler/terminator/conftest.py:142
TerminatingSessionData(fromai.backend.manager.repositories.scheduler.types.session) still defines the field asaccess_key, notmain_access_key. This fixture will fail to instantiate the dataclass. Update the fixture to use the current field name (or update the underlying type in the same PR if the rename is intended here).
return TerminatingSessionData(
session_id=session_id or SessionId(uuid4()),
access_key=AccessKey("test-key"),
creation_id=str(uuid4()),
status=SessionStatus.TERMINATING,
tests/unit/manager/sokovan/scheduler/handlers/conftest.py:548
ScheduledSessionDatastill defines the field asaccess_key, notmain_access_key(seerepositories/scheduler/types/results.py). This factory will fail to constructScheduledSessionDatauntil the type rename happens.
scheduled_sessions = [
ScheduledSessionData(
session_id=s.session_info.identity.id,
creation_id=s.session_info.identity.creation_id,
access_key=AccessKey(s.session_info.metadata.access_key),
tests/unit/manager/sokovan/scheduler/handlers/conftest.py:567
SessionDataForPullstill defines the field asaccess_key, notmain_access_key(seesokovan/data/lifecycle.py). This factory will fail to instantiateSessionDataForPullwith the renamed argument.
sessions_for_pull = [
SessionDataForPull(
session_id=s.session_info.identity.id,
creation_id=s.session_info.identity.creation_id,
access_key=AccessKey(s.session_info.metadata.access_key),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rename UserPermission.user_uuid to owner_id and add main_access_key: str | None. KernelRow.to_kernel_info populates the new field from the eagerly-loaded user_row relationship; search_kernels now selectinload's user_row so the scalar is available without N+1. KernelRow.delegate_ownership drops its access_key argument (resolved from the owner). recalc_concurrency_used switches from KernelRow.access_key to a KernelRow.user_uuid subquery keyed by the owner's main_access_key as a shim for the eventual access_key column drop. Test fixtures that construct UserPermission are updated.
Earlier the branch pulled test fixtures that also referenced later slices' renames (SessionMetadata.owner_id, ScheduledSessionData .main_access_key, SessionDataForPull/Start.main_access_key, SessionData.owner_id). Revert those fixtures to main state and re-apply only the UserPermission field renames (user_uuid -> owner_id, access_key -> main_access_key) which are in this slice's scope. Also update the two remaining live readers of UserPermission: - sokovan/scheduler/fair_share/aggregator.py - api/adapters/session.py (kernel_info_to_node) And fix the SessionRow.delegate_ownership caller of KernelRow.delegate_ownership to match the new single-argument signature.
jopemachine
force-pushed
the
refactor/BA-5650-B-user-permission-owner-id
branch
from
5ea1195 to
cfdf874
Compare
jopemachine
added
the
skip:changelog
jopemachine
removed
the
skip:changelog
Member
Author
|
Consolidated into the merged PR stack (A+B+C+D, E+F, G+H, I) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📚 Stack
mainMerge from top to bottom. Intermediate slices may not build standalone; the final refactor tip is #11051, and #11040 is the schema-drop follow-up on top of it.
Summary
Rename
UserPermission.user_uuid→owner_id; addmain_access_key: str | Nonefield.KernelRow.to_kernel_infopopulates it from the eagerly-loadeduser_rowrelationship;search_kernelsaddsselectinload.KernelRow.delegate_ownershipdropsaccess_keyarg.recalc_concurrency_usedswitches to auser_uuidsubquery shim.Resolves BA-5710. Part of epic BA-5650.
📚 Documentation preview 📚: https://sorna--11043.org.readthedocs.build/en/11043/
📚 Documentation preview 📚: https://sorna-ko--11043.org.readthedocs.build/ko/11043/
BA-5650 Series: Split Rationale
Overall goal: migrate the session owner identifier from
access_key(keypair) toowner_id(user UUID), and drop thesessions.access_key/kernels.access_keycolumns.Split criteria: layer + dependency order. Bottom-up (DB helpers → service → API) so the destructive column drop can land safely at the end.
get_main_access_key_by_idand related resolver helpers (everything else depends on this)UserPermission.user_uuid → owner_id; addmain_access_keyfieldSessionData.user_uuid → owner_id; Row adapters; GQL nodeSessionRepository,SessionDBSource, creators signaturesowner_idfrom 21 read/control Actions; resolve viacurrent_user()owner_access_keyfrom REST v1 DTOs (breaking)Why this split
access_keyfromowner_id. BA-5653 (destructive) only runs once every reader has migrated off the dropped column.